[client] add flag to force device code auth - #6885
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughAdds ChangesDevice authentication login
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change adds an optional device-code authentication flag to the CLI, and no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant LoginCommand
participant LoginRequest
participant ServerLogin
participant OAuthFlow
LoginCommand->>LoginRequest: Set UseDeviceAuth from CLI flag
LoginRequest->>ServerLogin: Send login request
ServerLogin->>OAuthFlow: Create or reuse matching auth mode
OAuthFlow-->>ServerLogin: Select device or browser authentication
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the feature, identifies tests, and provides a documentation PR link. It does not include the required issue ticket or approved discussion link, and it omits the required Stack section. Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 Buf (1.72.0)client/proto/daemon.protofatal: unable to access 'https://github.com/netbirdio/netbird.git/': Failed to connect to github.com port 443 via 127.0.0.1 after 0 ms: Could not connect to server Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Got no feedback on #6475 after an entire month even tho I even asked some of my colleagues (that also depend on this feature) to upvote it. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/server/server.go`:
- Line 587: Update the OAuth flow reuse logic near auth.NewOAuthFlow to include
msg.GetUseDeviceAuth() in the cached-flow identity, so browser and device-auth
flows with the same client ID are not reused interchangeably. Preserve reuse
only when both the client ID and authentication mode match, or invalidate the
cached flow when the mode changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 19532a32-4194-44f8-be8c-a15d9fdbd837
⛔ Files ignored due to path filters (2)
client/proto/daemon.pb.gois excluded by!**/*.pb.goclient/proto/daemon_grpc.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (5)
client/cmd/login.goclient/cmd/up.goclient/internal/auth/oauth_test.goclient/proto/daemon.protoclient/server/server.go
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/server/server.go (1)
626-643: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winCancel the previous pending flow before replacing it.
When the requested auth mode differs from the cached flow, the reuse condition is false and execution proceeds to create a new flow, but
s.oauthAuthFlow.waitCancelis not invoked. The old waiter can continue running whiles.oauthAuthFlowhas already been overwritten with the new flow. Cancel any active previous flow whenever replacing it, including on auth-mode changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/server/server.go` around lines 626 - 643, Ensure any active previous OAuth flow is canceled before creating or assigning a replacement, including when the requested auth mode differs. Update the flow replacement logic around s.oauthAuthFlow.waitCancel so cancellation occurs whenever the existing flow will not be reused, while preserving the current reuse behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@client/server/server.go`:
- Around line 626-643: Ensure any active previous OAuth flow is canceled before
creating or assigning a replacement, including when the requested auth mode
differs. Update the flow replacement logic around s.oauthAuthFlow.waitCancel so
cancellation occurs whenever the existing flow will not be reused, while
preserving the current reuse behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7c2b7b72-072e-4385-bc55-840a31e28e23
📒 Files selected for processing (1)
client/server/server.go
…th-merge-main # Conflicts: # client/cmd/login.go # client/proto/daemon.pb.go # client/proto/daemon.proto # client/server/server.go
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|



Describe your changes
Add a flag to cli up & login commands to force client to use device code flow.
Checklist
Documentation
Select exactly one:
Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:
netbirdio/docs#883
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit
--use-device-authflag when starting the service.